[metal] explicitly retain resources used by command buffers#8694
[metal] explicitly retain resources used by command buffers#8694uael wants to merge 2 commits intogfx-rs:trunkfrom
Conversation
This reverts commit 402a479.
When `retain_command_buffer_references` is false (the default), Metal's command buffers don't automatically retain the resources they reference. Previously, this was "fixed" by setting `retain_command_buffer_references` to true, but since command buffers are pooled and reused in wgpu, this caused unbounded memory growth - every resource ever touched by a command buffer stayed alive indefinitely. This commit implements proper resource lifetime tracking: - Track buffers and textures used during command encoding - Transfer references to CommandBuffer when encoding completes - Release references when command buffer is recycled
|
Tentatively assigning to @andyleiserson, who handled #7842. |
This is obviously bad, but I'm surprised we hadn't noticed. Do you have a test case? I don't think it should be necessary to add new tracking for used textures and buffers. We already have a "tracker" that is keeping track of them. The problem was that there are corner cases where we don't get it quite right. Vulkan is also sensitive to this issue and doesn't have any ability to track references internally, which has motivated fixing a bunch of the issues. When we turn the retain references flag back off, Metal will also benefit from those fixes. #7854 (comment) describes one case where we suspected Metal has a problem that differs from Vulkan. It would be worth running the test case from the issue that comment links to (#7816) against whatever change we're considering, to make sure the issue doesn't come back. |
|
To give a bit more context we got this crash on v25: As an attempt to fix it I blindly back-ported #7842, which resulted to "leaking" resources as explained above. Maybe I was just missing #8220 ? |
|
Closing in favor of #8706 |
3 participants
Description
Both revert #7842 and propose a proper fix by explicitly retaining resources used by command buffers.
When
retain_command_buffer_referencesis false (the new default after the revert), Metal's command buffers don't automatically retain the resources they reference.Previously, this was "fixed" by setting
retain_command_buffer_referencesto true, but since command buffers are pooled and reused, this caused unbounded memory growth - every resource ever touched by a command buffer stayed alive indefinitely.This commit implements proper resource lifetime tracking:
Testing
Manually
Checklist
cargo fmt.Runtaplo format.cargo clippy --tests. If applicable, add:--target wasm32-unknown-unknowncargo xtask testto run tests.If this contains user-facing changes, add aCHANGELOG.mdentry.